Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

build: fix some TS build issues in templates #2554

Closed
wants to merge 1 commit into from
Closed

build: fix some TS build issues in templates #2554

wants to merge 1 commit into from
+10 −7

Conversation

Merott
Copy link

@Merott Merott commented Nov 28, 2018

Fixes some of the typing issues found when ng-zorro is used in projects with TS strict mode, mostly in HTML templates.

Partially addresses #660, but still a long way to go.

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

[x] Bugfix
[ ] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[x] Build related changes
[ ] CI related changes
[ ] Documentation content changes
[ ] Application (the showcase website) / infrastructure changes
[ ] Other... Please describe:

What is the current behavior?

Issue Number: #660

Including ng-zorro in projects with TS strict mode on results in build errors, such as these:

ERROR in node_modules/ng-zorro-antd/ng-zorro-antd.d.ts.ɵeu.html(35,3): : Object is possibly 'null'.
node_modules/ng-zorro-antd/ng-zorro-antd.d.ts.ɵiv.html(3,3): : Object is possibly 'undefined'.
node_modules/ng-zorro-antd/ng-zorro-antd.d.ts.ɵiv.html(2,3): : Object is possibly 'undefined'.

What is the new behavior?

Fixes some of the typing issues in HTML templates. Issues such as the ones described above are resolved.

Does this PR introduce a breaking change?

[ ] Yes
[x] No

Other information

@codecov
Copy link

codecov bot commented Nov 28, 2018 •
edited
Loading

Codecov Report

Merging #2554 into master will increase coverage by 0.04%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2554      +/-   ##
==========================================
+ Coverage   95.57%   95.61%   +0.04%     
==========================================
  Files         507      495      -12     
  Lines       12086    12075      -11     
  Branches     1701     1687      -14     
==========================================
- Hits        11551    11546       -5     
+ Misses        171      169       -2     
+ Partials      364      360       -4
Impacted Files Coverage Δ
components/message/nz-message-config.ts 100% <ø> (ø) ⬆️
components/table/nz-th.component.ts 99.34% <100%> (ø) ⬆️
components/pagination/nz-pagination.component.ts 99.3% <100%> (ø) ⬆️
components/input/nz-autoresize.directive.ts 90.24% <0%> (-0.24%) ⬇️
components/tooltip/nz-tooltip.directive.ts 96.22% <0%> (-0.21%) ⬇️
components/layout/nz-sider.component.ts 91.07% <0%> (-0.16%) ⬇️
components/dropdown/nz-dropdown.component.ts 94.79% <0%> (-0.11%) ⬇️
...ponents/auto-complete/nz-autocomplete.component.ts 96.38% <0%> (-0.09%) ⬇️
components/popconfirm/nz-popconfirm.component.ts 100% <0%> (ø) ⬆️
components/popover/nz-popover.component.ts 100% <0%> (ø) ⬆️
... and 24 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6554cf5...156b876. Read the comment docs.

@netlify
Copy link

netlify bot commented Nov 28, 2018 •
edited
Loading

Deploy preview for ng-zorro-master ready!

Built with commit 156b876

https://deploy-preview-2554--ng-zorro-master.netlify.com

@Merott
Copy link
Author

Merott commented Dec 13, 2018

@vthinkxie any thoughts on this?

@Merott Merott closed this Dec 13, 2018
@Merott Merott deleted the fix-html-errors branch December 13, 2018 21:13
@Merott Merott restored the fix-html-errors branch December 13, 2018 21:14
@Merott Merott reopened this Dec 13, 2018
vthinkxie
vthinkxie reviewed Dec 28, 2018
View reviewed changes
components/message/nz-message-config.ts Outdated
@@ -7,6 +7,9 @@ export interface NzMessageConfig {
nzAnimate?: boolean;
// For message container only
nzMaxStack?: number;
nzTop?: string;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nzMessage does not support nzTop | nzBottom | nzPlacement? any reason add these properties here?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was because of nz-notification-container.component.html using those properties, although I just realised that component is working with NzNotificationConfig, which extends NzMessageConfig.

I've updated the PR to reflect that in NzNotificationContainerComponent, and I've also rebased it on master.

@Merott
build: fix some TS build issues in templates
156b876 
Fixes _some_ of the issues found when ng-zorro is used in projects with TS strict mode, mostly in HTML templates.

Partially addresses NG-ZORRO#660, but still a long way to go.
@vthinkxie
Copy link
Member

Hi @Merott
we will handle all the tsconfig error in #2884
thanks for your pr a lot!

@vthinkxie vthinkxie closed this Feb 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vthinkxie vthinkxie vthinkxie left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Merott @vthinkxie